Skip to content

fix(checkpoints): snapshot the task/session cwd before destructive terminal commands - #70230

Open
KikeTorillo wants to merge 1 commit into
NousResearch:mainfrom
KikeTorillo:fix/terminal-checkpoint-session-cwd
Open

KikeTorillo wants to merge 1 commit into
NousResearch:mainfrom
KikeTorillo:fix/terminal-checkpoint-session-cwd

Conversation

@KikeTorillo

Copy link
Copy Markdown

Problem

The checkpoint taken before a destructive terminal command resolves its directory as:

cwd = function_args.get("workdir") or os.getenv("TERMINAL_CWD", os.getcwd())

But the terminal tool itself executes the command in the task/session cwd (explicit workdir → per-task override → live session cwd → TERMINAL_CWD/process cwd — see terminal_tool's resolution). Any session created with an explicit cwd (session.create(cwd=...)) or re-anchored via session.cwd.set runs its commands in a directory that can differ from the Hermes process cwd.

Consequence: for those sessions, the pre-destructive-command checkpoint snapshots the wrong tree (the process cwd), and a later rollback silently restores nothing of what the rm/git reset/etc. actually touched.

_ensure_file_checkpoint was already fixed to honor the task cwd in #68195 — this is the terminal twin of that fix.

Fix

New _terminal_checkpoint_cwd(function_args, effective_task_id) helper that mirrors the file-tools resolution (_resolve_path_for_task(".", task_id) — same base-dir pipeline), used by both the batch and sequential call-sites. Explicit workdir still wins; without a session override, behavior is unchanged (TERMINAL_CWD → process cwd).

Tests

tests/agent/test_terminal_checkpoint_cwd.py:

  • explicit workdir wins,
  • a per-task cwd override beats process cwd and a stale TERMINAL_CWD (the regression),
  • without an override the historical TERMINAL_CWD fallback is preserved.

Found while integrating a downstream client that re-anchors sessions per workspace via session.cwd.set — file-tool checkpoints landed in the right tree, terminal ones didn't.

🤖 Generated with Claude Code

https://claude.ai/code/session_017PkWV7rR2oeg8BBttcNoyE

…rminal commands

The pre-destructive-command checkpoint resolved TERMINAL_CWD/os.getcwd()
directly, while the terminal tool itself executes in the task/session cwd
(explicit workdir -> per-task override -> live session cwd -> TERMINAL_CWD).
For sessions created with an explicit cwd (session.create(cwd=...)) or
re-anchored via session.cwd.set, the checkpoint snapshotted the WRONG tree
and the later rollback silently restored nothing.

_ensure_file_checkpoint was already fixed to honor the task cwd (NousResearch#68195);
this routes the terminal twin through the same file-tools path pipeline
(_resolve_path_for_task) via a shared helper, covering both the batch and
sequential call-sites.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017PkWV7rR2oeg8BBttcNoyE
@alt-glitch alt-glitch added type/bug Something isn't working comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P2 Medium — degraded but workaround exists sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Jul 23, 2026
@teknium1

Copy link
Copy Markdown
Collaborator

Thanks for identifying the terminal/file checkpoint divergence. The underlying issue is still present on current main: agent/tool_executor.py:615-624 snapshots TERMINAL_CWD/process cwd, while tools/terminal_tool.py:2164-2181 executes using the session cwd record.

Problems

  • The two checkpoint sites changed by this PR were consolidated into shared _begin_tool_execution by 14bed44c8c; the PR does not apply to current main as written.
  • The new tests exercise only the proposed helper, not the current preflight path at agent/tool_executor.py:536-626, so they would not validate the live checkpoint behavior after salvage.

Suggested changes

  • Move the fix into the shared preflight path and add an integration-style regression that captures both the checkpoint cwd and terminal execution cwd for a session-recorded workspace.
  • Preserve the explicit workdir and configured TERMINAL_CWD fallback cases.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform area/sessions Session lifecycle, resume, persistence, history labels Jul 30, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/sessions Session lifecycle, resume, persistence, history comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P2 Medium — degraded but workaround exists sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants